This is a comment left during a code review.
Path: pi/extensions/heartbeat.ts
Line: 614-655

Comment:
**Bridge killed before verifying restart is possible**

The bridge session and port 7890 are killed *before* verifying that either the startup script exists or that there are live socket UUIDs to pass it. If `startupScript` doesn't exist, or `sockFiles` is empty (no `.sock` files), the code falls through both inner `if` guards without pushing any `RecoveryAction` entry and without logging anything. The bridge is now dead but `recoveryActions` contains no entry for `"bridge"`, so:

1. `recoveredChecks` will NOT contain `"bridge"`.
2. The bridge failure remains in `remainingFailures` — the agent is prompted.
3. But the agent has no idea the previous bridge tmux session was already killed and port 7890 was cleared — it's working against a ghost.

A minimal fix is to push a "partial failure" action in the missing branches:

```typescript
// After killing bridge/port but before the startup script check:
if (!existsSync(startupScript)) {
  const entry: RecoveryAction = {
    timestamp: new Date().toISOString(),
    check: failure.name,
    action: "bridge_restart",
    success: false,
    detail: `Bridge session killed but startup script not found at ${startupScript} — cannot restart`,
  };
  actions.push(entry);
  logRecovery(entry);
  continue;
}

// …and similarly if uuids is empty:
if (!uuids) {
  const entry: RecoveryAction = {
    timestamp: new Date().toISOString(),
    check: failure.name,
    action: "bridge_restart",
    success: false,
    detail: "Bridge session killed but no live socket UUIDs found — cannot restart",
  };
  actions.push(entry);
  logRecovery(entry);
  continue;
}
```

This ensures the agent prompt always includes the full picture of what auto-recovery attempted.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: pi/extensions/heartbeat.ts
Line: 676

Comment:
**Shell injection via unsanitized session name from filesystem**

`sessionName` is derived directly from filenames in `SOCKET_DIR` (e.g. a file named `dev-agent-foo"; rm -rf ~; echo ".alias`). While the directory is inside `~/.pi/`, the shell interpolation via template literal in `execSync` would execute arbitrary injected commands if such a filename existed.

Prefer the array-argument form of `spawnSync` / `execFileSync` which bypasses the shell entirely:

```typescript
import { execFileSync } from "node:child_process";
// …
execFileSync("tmux", ["kill-session", "-t", sessionName], { timeout: 5000 });
```

Similarly on line 635 for the startup script invocation — the UUIDs should each be passed as separate shell-quoted arguments, not space-joined into a single string.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: pi/extensions/agent-spawn.ts
Line: 544-548

Comment:
**Dynamic `require()` in an ESM/TypeScript file — use `readFileSync` instead**

This file uses static ES module `import` declarations at the top, but `spawn_status` falls back to `require("node:child_process")` and a shell `tail` command to read the last 20 lines of the JSONL log. Two problems:

1. Dynamic `require()` is a CommonJS pattern and can be surprising in an ESM context.
2. Shelling out to `tail` for a simple file-tail operation is non-portable (unavailable on Windows, behaves differently under PATH changes) and redundant — `readFileSync` is already available as a static import in this file.

A portable alternative using the already-imported `readFileSync`:

```typescript
import { appendFileSync, existsSync, mkdirSync, readFileSync, readlinkSync, statSync } from "node:fs";
// …
// Inside spawn_status execute():
let recentEvents = "";
try {
  if (existsSync(LIFECYCLE_LOG_PATH)) {
    const lines = readFileSync(LIFECYCLE_LOG_PATH, "utf-8")
      .trimEnd()
      .split("\n")
      .slice(-20);
    // … same parse/format logic
  }
} catch {
  recentEvents = "  (no lifecycle log)";
}
```

The same `require("node:child_process")` / `require("node:fs")` pattern also appears in `heartbeat.ts` (lines 595, 684) where top-level static imports should be used instead.

How can I resolve this? If you propose a fix, please make it concise.